-
Notifications
You must be signed in to change notification settings - Fork 8.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[APM] Fix broken e2e tests #129475
[APM] Fix broken e2e tests #129475
Conversation
.type(moment(start).format(format), { force: true }); | ||
cy.get('[data-test-subj="superDatePickerendDatePopoverButton"]').click(); | ||
cy.get('[data-test-subj="superDatePickerAbsoluteDateInput"]') | ||
.eq(1) | ||
.clear() | ||
.clear({ force: true }) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This element is not visible and the type
below results in invalid date
cy.contains('Day before'); | ||
cy.contains('Week before'); | ||
|
||
cy.selectAbsoluteTimeRange( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed this to use absolute dates to calculate the period
consistently.
...ins/apm/ftr_e2e/cypress/integration/read_only_user/service_overview/service_overview.spec.ts
Show resolved
Hide resolved
}); | ||
|
||
it('with the correct environment when changing the environment', () => { | ||
cy.wait(aliasNames); | ||
cy.wait(aliasNames, { requestTimeout: 10000 }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even with 10000
they fail sometimes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any idea why? is it because they finish slightly later? or they're not captured at all?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't managed to reproduce it locally and can't tell from the CI failure
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm, is there any way we can figure it out? maybe add & log some request timings for the time being?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've increased the requestTimeout
to 30s and temporarily removed cypress retries in 761dc8e and in 50 test runs it doesn't seem to have failed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually one is still failing :|
cy.visit(serviceInventoryHref); | ||
|
||
cy.contains('Services'); | ||
cy.contains('opbeans-rum').wait(3000).click({ force: true }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Waiting on DOM elements is discouraged but click
causes the test to be flaky. As we've seen this quite a few times so far, we might benefit by using cypress-pipe (also used by security)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't we wait on API calls to complete?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could but not sure if this will change it. opbeans-rum
is there, it's just the click that is failing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it because the table is still in a loading state?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be, also I've read that some DOM-related flaky tests might be due to components re-rendering after cy.contains
and before click
causing such issues.
@@ -47,12 +41,12 @@ describe.skip('Home page', () => { | |||
cy.loginAsReadOnlyUser(); | |||
}); | |||
|
|||
it('Redirects to service page with environment, rangeFrom and rangeTo added to the URL', () => { | |||
it('Redirects to service page with comparisonEnabled, environment, rangeFrom, rangeTo and offset added to the URL', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More redirects have been added by #127812
@@ -65,6 +65,7 @@ describe('Infrastracture feature flag', () => { | |||
it('shows infrastructure tab in service overview page', () => { | |||
cy.visit(serviceOverviewPath); | |||
cy.contains('a[role="tab"]', 'Infrastructure').click(); | |||
cy.contains('Infrastructure data coming soon'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An expectation was added to be more explicit
...s/apm/ftr_e2e/cypress/integration/read_only_user/service_inventory/service_inventory.spec.ts
Show resolved
Hide resolved
Pinging @elastic/apm-ui (Team:apm) |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
|
||
// flaky test | ||
describe.skip('Home page', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we unskip this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is still flaky
@@ -77,11 +75,14 @@ describe('When navigating to the service inventory', () => { | |||
cy.contains('h1', 'opbeans-node'); | |||
}); | |||
|
|||
describe('Calls APIs', () => { | |||
describe.skip('Calls APIs', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you leave a comment here to explain why it's skipped (and maybe reference an issue)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, it was failing while waiting for the API calls to finish
describe('Filtering by transaction type', () => { | ||
beforeEach(() => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what's the purpose of moving this into both child test suites? was this supposed to be a before()
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dgieselaar I noticed that once in a while a test fails because cypress logs kibana out, so to avoid such a thing I decided to move the login function to inside each describe
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some comments about some remaining skipped tests, but LGTM other than that
Thanks for having a look, it seems that it can still be flaky |
wow, that's so weird, it passed 50 times on my env. |
@elasticmachine merge upstream |
💛 Build succeeded, but was flakyTest FailuresMetrics [docs]
History
To update your PR or re-run it, just comment with: |
* Fix skipped e2e tests * Add expectation for the infrastructure tab * Add option to run APM cypress tests in the flaky test runner Co-authored-by: cauemarcondes <caue.marcondes@elastic.co> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> (cherry picked from commit f0553d3)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
* Fix skipped e2e tests * Add expectation for the infrastructure tab * Add option to run APM cypress tests in the flaky test runner Co-authored-by: cauemarcondes <caue.marcondes@elastic.co> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> (cherry picked from commit f0553d3) Co-authored-by: Giorgos Bamparopoulos <georgios.bamparopoulos@elastic.co>
Summary
x-pack/plugins/apm/ftr_e2e/cypress/integration/read_only_user/home.spec.ts
x-pack/plugins/apm/ftr_e2e/cypress/integration/read_only_user/service_overview/service_overview.spec.ts
x-pack/plugins/apm/ftr_e2e/cypress/integration/read_only_user/service_overview/time_comparison.spec.ts
Addresses part of #128752